-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate Azure MSI & Metadata Service token sources to httpclient
and add 100% test coverage
#709
Conversation
8ac91f9
to
fdc7906
Compare
httpclient
and add 90% test coveragehttpclient
and add 100% test coverage
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #709 +/- ##
==========================================
+ Coverage 16.86% 17.57% +0.71%
==========================================
Files 103 103
Lines 14128 14188 +60
==========================================
+ Hits 2383 2494 +111
+ Misses 11536 11489 -47
+ Partials 209 205 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good, a couple comments/suggestions/questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After offline discussion, LGTM, please add docs for that one parameter and feel free to merge.
This PR improves the stability for Azure MSI authentication by adopting the httpclient transport. add 100% coverage for MSI & metadata-service .. .. ..
fa1ff7d
to
3e96fd4
Compare
tested as |
This is a bugfix release, including a fix correcting issues with OAuth flows, due to a bug with the propagation of the response status in `httpclient`'s `RoundTrip()` implementation. This fixes the `failed during request visitor: token: oauth2: cannot fetch token: Response: {...}` error. All fixes: * Migrate Azure MSI & Metadata Service token sources to `httpclient` and add 100% test coverage ([#709](#709)). * Added `config.NewAzureCliTokenSource` and `config.NewAzureMsiTokenSource` constructors ([#727](#727)). * Use per-config refresh context for OAuth tokens ([#728](#728)).
This is a bugfix release, including a fix correcting issues with OAuth flows, due to a bug with the propagation of the response status in `httpclient`'s `RoundTrip()` implementation. This fixes the `failed during request visitor: token: oauth2: cannot fetch token: Response: {...}` error. All fixes: * Migrate Azure MSI & Metadata Service token sources to `httpclient` and add 100% test coverage ([#709](#709)). * Added `config.NewAzureCliTokenSource` and `config.NewAzureMsiTokenSource` constructors ([#727](#727)). * Use per-config refresh context for OAuth tokens ([#728](#728)).
This PR improves the stability for Azure MSI authentication by adopting the httpclient transport.
Needs two PRs merged first:
qa.HTTPFixtures
to faster transport-level stubs #708DatabricksEnvironment
and fix Azure MSI auth from ACR, where IMDS doesn't give host environment information #700